feat(task): add path-prefix batch cancel, delete and retry - #2846
feat(task): add path-prefix batch cancel, delete and retry#2846ifloppy wants to merge 2 commits into
Conversation
- Add `TaskWithPaths` interface and `MatchTaskPath` helper for matching tasks by virtual path prefix - Expose `GetSrcPath`/`GetDstPath` on transfer, upload, download and archive tasks - Add `delete_by_path`, `cancel_by_path` and `retry_by_path` endpoints for every task type - Cancel unfinished tasks before removing them so queued work stops - Scope matching to the requesting user and their base path - Exclude local temp and URL sources from path matching - Add unit and HTTP-level tests covering matching, permissions and large task sets Signed-off-by: ifloppy <68799904+ifloppy@users.noreply.github.com>
xrgzs
added
enhancement
Module: Task
|
I reviewed the path matching, permission boundaries, state filtering, and test coverage. The overall direction looks reasonable, but I have several concerns before this can be merged.
For normal uploads, the destination path omits the uploaded file name. Archive-content uploads similarly omit the object name. As a result, filtering by an exact destination file path will not match the corresponding task, even though the API describes this as matching the task's source or destination path. Please either include the final object name in these paths and add tests for exact-file matching, or explicitly define and document that these task types use destination-directory semantics.
Please add a test with a genuinely running/blocking task that verifies cancellation is observed, execution stops, cleanup completes, and the task is then removed without continuing in the background.
Inputs such as Please require an explicit
The returned count currently represents tasks selected by the initial filter, not necessarily operations that completed successfully. Concurrent task changes can make
Please add or link API documentation covering path normalization, source-or-destination matching, user scoping, root-path behavior, state filtering, and count semantics. The existing tests cover matching boundaries, ownership, traversal attempts, state filtering, and scale well. Once the lifecycle and path-contract questions above are addressed, the backend and OpenList-Frontend#608 should be reviewed and merged as one coordinated change. |
- Match upload and archive-content tasks by their final destination object path - Require an explicit slash for operations that resolve to the global root - Wait for running task cancellation and cleanup before removing tasks - Return separate matched and processed counts for concurrent state changes - Document normalization, scoping, state filters, lifecycle and response semantics - Cover exact-file matching, lifecycle cleanup, implicit root rejection and count races Signed-off-by: ifloppy <68799904+ifloppy@users.noreply.github.com>
|
Thank you for the detailed review. I addressed each concern in follow-up commit e8879b3, with the corresponding frontend adjustment in OpenList-Frontend#608 commit ef01089.
Validation performed:
All of the above pass. The follow-up implementation and this response were AI-assisted with Claude, as disclosed in the updated PR AI Disclosure section, and the changes/tests were reviewed and validated before pushing. |
PIKACHUIM
left a comment
PIKACHUIM
left a comment
There was a problem hiding this comment.
🙏 感谢贡献
感谢 @ifloppy 提交此PR！我已完成代码评审，以下是评审结果。
🤖 AI 自动审核声明
本评审报告由 AI 自动生成，当前使用 Claude Opus 5 模型进行分析，部分复杂场景可能辅助使用 ChatGPT、DeepSeek 等模型进行交叉验证。
⚠️ AI 分析结果仅供参考，可能存在误判或遗漏。如您发现任何问题或有不同意见，欢迎随时提出讨论和纠正。
⚠️ 重要提醒：即使 AI 评审认为代码质量良好且建议合并，最终是否合并仍需由项目维护者进行人工判定。项目维护者会综合考虑代码质量、项目规划、技术方向、团队资源等多方面因素做出决策。
📖 PR背景与需求
PR标题：feat(task): add path-prefix batch cancel, delete and retry
需求说明：在任务管理中新增按路径前缀的批量操作功能，解决大量任务（上万条）场景下需要逐个勾选任务的问题。现有 API（delete_some、cancel_some）需要传入每个任务 ID，clear_done 无法按路径过滤，导致按目录清理任务难以实施。
预期目标：
- 为所有任务类型（upload/copy/move/offline_download/offline_download_transfer/decompress/decompress_upload）新增三个端点：
delete_by_path、cancel_by_path、retry_by_path - 通过路径前缀匹配任务，无需逐个传入任务 ID
- 区分"匹配数量"和"处理数量"，提供精确的操作反馈
📋 问题摘要
- ✅ 功能性：功能设计完整，解决实际痛点
- ✅ 安全性：权限控制严格，路径解析安全
- ✅ 代码质量：测试覆盖充分，实现精准
- 💡 改进建议：2处小优化点
📂 逐文件分析
docs/task-path-batch-api.md
改动意图：为新增的路径前缀批量 API 提供完整的文档说明。
代码修改逻辑：
- 说明了三个新端点的用途和使用方法
- 明确了路径匹配规则（精确匹配或子路径匹配，路径分段感知）
- 解释了响应中
matched和processed的区别 - 强调了根路径操作需要显式传入
/，防止误操作
合理性评估：
- ✅ 优点：
- 文档详细，包含请求示例、响应示例、匹配规则、权限说明
- 明确了安全边界（根路径需要显式传入，拒绝
.、./、//等模糊值） - 说明了并发情况下
matched和processed可能不一致的原因
internal/task/path.go & internal/task/path_test.go
改动意图：定义任务路径匹配的核心接口和逻辑。
代码修改逻辑：
- 新增
TaskWithPaths接口，要求任务实现GetSrcPath()和GetDstPath()方法 - 实现
MatchTaskPath函数，检查源路径或目标路径是否匹配指定前缀 - 使用
utils.IsSubPath判断子路径关系，确保/a不会匹配/ab - 测试覆盖完整（前缀匹配、精确匹配、路径分段、反斜杠清理、相对路径、空路径等）
合理性评估：
- ✅ 优点：
- 接口设计清晰，职责明确
- 路径匹配逻辑安全，防止误匹配（如
/avs/ab） - 测试用例全面，覆盖边界情况
internal/fs/archive.go、internal/fs/other.go、internal/fs/put.go、internal/offline_download/tool/download.go
改动意图：为各类任务实现 TaskWithPaths 接口，暴露源路径和目标路径。
代码修改逻辑：
- TaskData：从
SrcStorageMp/SrcActualPath拼接源路径，从DstStorageMp/DstActualPath拼接目标路径；当存储挂载点为空时返回空字符串（排除本地临时文件） - UploadTask：源路径为空（本地上传），目标路径为
存储挂载点 + 目标目录 + 文件名 - ArchiveContentUploadTask：源路径为空，目标路径根据
InPlace决定（InPlace=true时返回目标目录，false时加上对象名） - DownloadTask：源路径为空（下载 URL），目标路径为
DstDirPath
合理性评估：
- ✅ 优点：
- 实现一致，遵循"本地临时路径/URL 不参与匹配"的原则
- 目标路径精准到最终对象路径（包含文件名）
- 测试覆盖充分（97行 + 33行单元测试）
server/handles/task.go & 测试文件
改动意图：在 HTTP 层实现路径前缀批量操作的三个端点。
代码修改逻辑：
resolveTaskPathPrefix：解析用户输入的路径前缀，非管理员用户通过user.JoinPath解析到用户空间；拒绝模糊的根路径表示（如.、./、//）pathBatchOp结构体：封装批量操作的核心逻辑，支持delete/cancel/retry三种操作delete_by_path：先取消所有匹配的未完成任务，等待执行停止和清理完成（最多30秒），然后移除确认停止的任务cancel_by_path：仅取消未完成的任务，任务保留在管理器中以便查看最终状态retry_by_path：仅重试失败状态的任务- 权限控制：非管理员只能操作自己创建的任务，路径解析限制在用户空间
合理性评估：
- ✅ 优点：
- 两阶段删除（先取消，等待停止，再移除）安全可靠，防止数据损坏
- 状态过滤精准（
cancel只影响未完成任务，retry只影响失败任务） - 并发安全（使用锁保护共享状态）
- 测试覆盖全面（HTTP 层测试197行 + 生命周期测试333行）
⚠️ 疑问：taskDeleteWaitTimeout = 30 * time.Second对于大量任务可能不够。建议在文档中说明超时未停止的任务会保留在管理器中，需要手动处理。- 根路径操作的安全检查依赖
resolveTaskPathPrefix的拒绝逻辑，但没有二次确认机制。建议在响应中增加根路径操作的警告标记。
详细建议：
- 在文档中补充超时说明：
### Timeout behavior `delete_by_path` waits up to 30 seconds for active tasks to stop. Tasks that do not stop before the timeout remain visible in the manager and are not counted as processed. You can inspect these tasks manually or retry the delete operation after they finish.
- 考虑在响应中增加根路径操作警告：
type TaskPathResult struct { Matched int `json:"matched"` Processed int `json:"processed"` IsRootPath bool `json:"is_root_path,omitempty"` Warning string `json:"warning,omitempty"` }
🎯 总体评价
功能性：⭐⭐⭐⭐⭐ - 功能完整，精准解决大量任务场景下的批量操作痛点
安全性：⭐⭐⭐⭐⭐ - 权限控制严格，路径解析安全，根路径操作有防护
代码质量：⭐⭐⭐⭐⭐ - 测试覆盖充分（700+行测试），实现精准，文档详细
实现方案：⭐⭐⭐⭐⭐ - 两阶段删除安全可靠，状态过滤精准，并发安全
建议操作：
- ✅ Approve（建议合并）
- 🔄 Request Changes（需要修改）
- ❌ Close（建议关闭）
理由：这是一个教科书级别的功能实现 —— 功能设计完整、安全防护到位、测试覆盖充分、文档详细。建议的改进点（超时说明、根路径警告）是锦上添花，不阻碍合并。
Next Steps / 后续建议：
- 在文档中补充超时行为说明和根路径操作的安全提示
- 考虑在前端增加根路径操作的二次确认对话框
- 可以在后续版本中增加
offset/limit参数支持分页批量操作
再次感谢你的贡献！这个功能将显著提升大规模任务管理的用户体验。👏
4 participants
Summary / 摘要
Adds path-prefix batch operations for tasks, so large numbers of tasks can be cancelled, deleted or retried without selecting them one by one.
为任务增加按路径前缀的批量操作，使大量任务无需逐条勾选即可取消、删除或重试。
Motivation: with tens of thousands of queued tasks, the existing
delete_some/cancel_someendpoints require sending every task ID, andclear_donecannot filter by path. Cleaning up all tasks under one directory was impractical.动机：在有上万条排队任务时，现有
delete_some/cancel_some需要传入每个任务 ID，而clear_done无法按路径过滤，导致按目录清理任务难以实施。User-visible changes / 用户可感知的变化:
upload,copy,move,offline_download,offline_download_transfer,decompress,decompress_upload):POST /api/task/{type}/delete_by_pathPOST /api/task/{type}/cancel_by_pathPOST /api/task/{type}/retry_by_path{"path": "/prefix"}{"matched": N, "processed": M}delete_by_pathcancels all matching unfinished tasks first, waits for active execution and cleanup to finish, then removes only tasks confirmed safe to removecancel_by_pathonly affects unfinished tasks;retry_by_pathonly affects failed tasks/; ambiguous values such as.,./, or//are rejected if they resolve to/Implementation changes / 重要实现变化:
Added
task.TaskWithPathsinterface andtask.MatchTaskPathhelperImplemented final-object
GetSrcPath/GetDstPathvalues onTaskData,UploadTask,ArchiveContentUploadTaskandDownloadTaskAdded two-phase running-task deletion: cancel all, wait up to the shared deadline for terminal state/cleanup, then remove confirmed stopped tasks
Matching reuses
utils.IsSubPath, so/adoes not match/abLocal temp paths and offline-download URLs are excluded from matching to avoid unintended deletions
Non-admin requests are resolved through
user.JoinPath, and matching is limited to the requesting user's own tasksExisting endpoints are untouched
This PR has breaking changes.
/ 此 PR 包含破坏性变更。
This PR changes public API, config, storage format, or migration behavior.
/ 此 PR 修改了公开 API、配置、存储格式或迁移行为。
This PR requires corresponding changes in related repositories.
/ 此 PR 需要关联仓库同步修改。
Related repository PRs / 关联仓库 PR:
Testing / 测试
Commands run on Windows (
go1.26.4):go test ./internal/task/ ./internal/fs/ ./internal/offline_download/tool/ ./server/handles/ ./pkg/utils/— all passgo vetandgofmt -lon the changed packages — cleango build ./...— succeedsAutomated coverage added:
/avs/ab, backslash and relative path cleaning, empty path sides.,./,//, and repeated separators are rejected when they would resolve to global/; explicit/is acceptedcancel_by_pathskips terminal states,retry_by_pathonly touches failed taskshttptest): a non-admin cannot affect another user's tasks, and path traversal such as../../etc/secretcannot reach tasks outside the user's base pathRunexits and delayedOnFailedcleanup finishes, and the task remains inactive after removalmatched=1, processed=0Manual test / 手动测试:
linux/amd64binary with the frontend embedded and ran it on Linuxpath is requiredundone,done,clear_doneanddelete_somestill respond normallyNote:
go test ./...also reports pre-existing failures indrivers/189,drivers/chaoxing,drivers/google_drive,drivers/lanzou,pkg/aria2/rpcandinternal/net. These are unrelated to this change; I confirmed they reproduce identically with this branch's changes stashed (non-constant format stringvet checks from a newer Go, and tests requiring a local aria2 instance).Checklist / 检查清单
/ 我已阅读 CONTRIBUTING。
/ 我确认此贡献符合仓库许可证、贡献规范和行为准则。
gofmt,go fmt, orprettierwhere applicable./ 我已按适用情况使用
gofmt、go fmt或prettier格式化变更代码。/ 我已在适用情况下请求相关维护者或代码所有者审查。
AI Disclosure / AI 使用声明
/ 此 PR 包含 AI 辅助内容。
Tools used / 使用工具:
Usage scope / 使用范围:
Code generation / 代码生成
Refactoring / 重构
Documentation / 文档
Tests / 测试
Translation / 翻译
Review assistance / 审查辅助
I have reviewed and validated all AI-assisted content included in this PR.
/ 我已审核并验证此 PR 中的所有 AI 辅助内容。
I have ensured that all AI-assisted commits include
Co-Authored-Byattribution./ 我已确保所有 AI 辅助提交都包含
Co-Authored-By归属信息。I can reproduce all AI-assisted content included in this PR without any AI tools.
/ 我可以在没有任何 AI 工具的情况下重现此 PR 中包含的所有 AI 辅助内容。
The commits in this PR do not carry a
Co-Authored-Bytrailer. The AI assistance is disclosed here instead. Please let me know if you would prefer the trailer added to the commits, and I will amend them.此 PR 的提交未附带
Co-Authored-Bytrailer，AI 辅助情况改在此处声明。如维护者希望在提交中补充该 trailer，请告知，我会修改提交。